Added notification support for new items being added to a library #3464
Conversation
…pports the client upload method (one or more items), or folder changes.
docs/openapi.json
Outdated
There was a problem hiding this comment.
You will want to edit the file in docs/objects/Notification.yaml to add the new element to the NotificationEventName. The docs/openapi.json is automatically generated from those files and should not be edited directly.
There was a problem hiding this comment.
Oh, whoops. I misread the previous edit, disregard.
server/scanner/LibraryScanner.js
Outdated
| } | ||
| itemGroupingResults[itemDir] = newLibraryItem ? ScanResult.ADDED : ScanResult.NOTHING | ||
| } | ||
|
|
||
| return itemGroupingResults | ||
| } | ||
| } | ||
| module.exports = new LibraryScanner() | ||
| module.exports = new LibraryScanner(new NotificationManager()) |
There was a problem hiding this comment.
The server doesn't run because NotificationManager is not defined.
We wouldn't want to instantiate a new instance of NotificationManager though because it is already instantiated in Server.js.
The NotificationManager could be made a singleton like the LibraryScanner so it can be imported anywhere
There was a problem hiding this comment.
I updated it to a singleton so you can merge master to fix
7cd8d7f
There was a problem hiding this comment.
Interesting, I ran the server just fine with the new instantiation of NotificationManager here. Maybe I missed something when migrating my changes from a branch off master to my fork. Agreed, a singleton would have been better, but I didn't feel comfortable making a sweeping change like that, so I appreciate the enhancement. I will update accordingly.
|
The main reason I hesitated on adding this feature is what I described here: #996 (comment) When testing this I scanned in a new library and it queued up dozens of emails. For some users it could be 10k+ emails. I'm not sure the best way to handle that. The other issue I have with this is the notification is named |
These are valid concerns, I appreciate referencing the previous conversation for added context. Let me think about this and see if I can propose a potential solution that will help address these. There will be some UI changes needed to give the user some added control over the library, notification thresholds/limits (n > items consolidate into a single notification, restrict notification event to specific library(ies), be more specific about what kind of item: book, podcast, disable option when adding a new library, allow users to un/subscribe to notifications, potentially an option as an Admin to manually trigger a notification via the book's details page, etc.). I do think this is worth solving as there are many out there looking for this feature like I am - hence the motivation to try and contribute to its solution. I am currently wanting to send notification embeds to my private Discord server where my users are listening. I will see if I can document a full Feature specification for this that User Stories could be groomed from for design, estimation, impact/risk, and prioritization. Maybe others could assist in the development. |
Supports the client upload method (one or more items), or folder changes. I tested each scenario locally through docker and using Pipedream webhooks to receive the notification event.